Fix undefined behaviour in View::tryCopyTo overlap#1287
Merged
Simn merged 1 commit intoHaxeFoundation:masterfrom Jan 7, 2026
Merged
Fix undefined behaviour in View::tryCopyTo overlap#1287Simn merged 1 commit intoHaxeFoundation:masterfrom
Simn merged 1 commit intoHaxeFoundation:masterfrom
Conversation
Contributor
|
Ah, nice catch! I forgot that memcpy wasn't safe in overlaps and wasn't thinking about that case at all. |
Member
Author
|
Looks like ci got reran but just in case it comes up again, we had this failure during execution of the mac arm64 "haxe" test: |
ionosoft
pushed a commit
to ionosoft/hxcpp-ion
that referenced
this pull request
Mar 9, 2026
Picks up 37 upstream commits since our last merge (Sep 2, 2025), including: - Unconditional HXCPP_ALIGN_ALLOC (required for std::atomic in GC objects) - Missing GCFreeZone calls for semaphores/condvars (HaxeFoundation#1296) - C++11 std::mutex and condition variable for GC (HaxeFoundation#1298) - Callable objects with GC write barriers (HaxeFoundation#1047) - Root handle API (GCAddRoot/GCRemoveRoot) (HaxeFoundation#1292) - Fix undefined behaviour in View::tryCopyTo overlap (HaxeFoundation#1287) - Fix static target null Int problem - Skip splicing when no changes (HaxeFoundation#1309) Our ionosoft-specific changes preserved: - GcRegCapture RSI register fix (d71061e) - msvc-toolchain.xml debug build flags Co-Authored-By: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This was causing random failures in the haxe test suite: #1286 (comment)
The part where it was going wrong was here: https://github.com/HaxeFoundation/haxe/blob/3aed86e98aaa6097c3b8c9ab5c1aee5f70faa2f9/tests/unit/src/unit/TestBytes.hx#L47
The new implementation uses memcpy for this blit, which is undefined behaviour since the source and destination regions overlap. See: https://en.cppreference.com/w/cpp/string/byte/memcpy.html
std::memmoveis the alternative which is safe to use for overlapping blocks: https://en.cppreference.com/w/cpp/string/byte/memmove.html. This is also what is used in the ArrayBase::Blit method:hxcpp/src/Array.cpp
Lines 179 to 182 in 4cff17a